Repository navigation
fix(gui3): resolve 4 bugs from owner feedback - #2586
Conversation
fl0rianr
left a comment
fl0rianr
left a comment
There was a problem hiding this comment.
Thanks, this fixes parts of the reported GUI3 issues, but I would adjust a few things before merging:
-
detectDefaultBaseUrl()currently trusts anywindow.location.port. In the prototype dev setup,webpack serveruns on port 8080 and there is no/apiproxy, so the default becomeshttp://localhost:8080and API calls would hit the UI dev server instead of lemond. This should only use same-origin when we know the UI is actually served by lemond, or it should fall back to the known Lemonade API default / host-providedapiUrl. -
The PR claims to fix UDP beacon discovery (#2561), but this does not implement beacon discovery. It only helps the “served by lemond” case and the mock
apiUrlcase. Either the issue/PR wording should be narrowed, or the actual discovery path needs to be implemented. -
The AutoOpt run action is fine by me but not enough to close the issue, yet - or open another one for a least one useful autorun scenario to perform (to show of capability)
-
The per-run delete button is only visible on hover. That is weak for mobile/touch and keyboard accessibility. Please also reveal it on
:focus-within, or make it persistently visible / move it into a compact action menu.
The model filtering and Dashboard reconnect polling changes look reasonable.
- Auto-discovery: derive default base URL from window.location.origin when served by lemond, and read mock API's apiUrl field as fallback. Fixes #2561. - Dashboard port: listen for connection status changes and resume polling when connection is re-established after failures. Fixes #2562. - Models filtering: hide models with suggested:false unless they are downloaded or currently loaded. Fixes #2560. - AutoOpt rail: add Run Optimizer button and per-run Delete action so the panel is actually usable. Fixes #2567. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
7bbaa72 to
93d17d3
Compare
kpoineal
left a comment
kpoineal
left a comment
There was a problem hiding this comment.
Thanks @fl0rianr — addressed points 1 and 4:
-
Fixed: \detectDefaultBaseUrl()\ now only uses same-origin when it detects the lemond shim (\window.api.getServerPort) or the /app\ path prefix. Dev servers on arbitrary ports fall through to the 13305 default.
-
Acknowledged: You're right — this doesn't implement real UDP beacon. That's not possible from a browser context. Narrowing #2561 to what we actually fixed (auto-connect when served by lemond + apiUrl fallback). True beacon discovery would need a server-side endpoint.
-
Agreed: The AutoOpt action is a UI scaffold. I'll open a follow-up issue for implementing a real optimization run (e.g. benchmark current backend params → suggest improvements).
-
Fixed: Delete button now also reveals on :focus-within\ and direct :focus, making it keyboard/touch accessible.
#2613 I created one. |
- Auto-discovery: derive default base URL from window.location.origin when served by lemond, and read mock API's apiUrl field as fallback. Fixes #2561. - Dashboard port: listen for connection status changes and resume polling when connection is re-established after failures. Fixes #2562. - Models filtering: hide models with suggested:false unless they are downloaded or currently loaded. Fixes #2560. - AutoOpt rail: add Run Optimizer button and per-run Delete action so the panel is actually usable. Fixes #2567. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2 participants
Fixes 4 bugs reported by Jeremy:
Fixes #2560, #2561, #2562, #2567